-
-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perception publish tags as a world pose #25
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine
|
I thought we weren't gonna make SE3 inherit from Pose, also we should make sure the python and cpp interfaces are identical |
Yes I agree on both points. How do you suggest we store information for the former? IIRC we were going to use a 3 tuple and a 4 tuple for position and rotation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. Looks good, mostly nits, a few substantial things. Would also like to see some more detail on the wiki for the logic of this code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, looks good to me, though I'm still learning about ROS and percep code. Can give another pass after you've addressed the rest of Ashwin's comments
For some reason I can't respond to this inline, but yes exactly - the point was to just show that those topics were being used. I actually think we should enforce this, that is something I would like to discuss. |
Okay so I made some major changes to the way all the transform stuff is calculated, much better now imho. It only uses the tf tree for calculations and avoids any manual matrix manipulation. The SE3 class is more flushed out now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but still won't work with real ZED until you get Jason's fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SE3 implementation here is good enough for now until we figure out the interface fully, we can push the thing about remap in launch files to #43
* Refactor perception * Continue working on sending tags as pose * Remove marker msg, use vectors instead * Rename member variables, move to using a struct * Revert changes b/c clion thought it was too smart * Use map instead of vector so we can keep track even if they go out of sight * Fix launch files, remove dead code * Split processing into separate file * Add SE3, add filter, slightly modify clang format, restructure perception * Filtered positioning for arucos in 3D space working, refactoring * Fix formatting * Refactor out SE3 transform function * Add more to SE3, continue refactor * Name median mean filter, cleanup se3 names * Use eigen for implementation of se3 * Use tf tree instead of matrix multiplication * Minor documentation * Update README.md
* Update CMakeLists.txt * Add msg/ files into CMakeLists.txt * delete vscode Co-authored-by: wilchen <[email protected]>
* Refactor perception * Continue working on sending tags as pose * Remove marker msg, use vectors instead * Rename member variables, move to using a struct * Revert changes b/c clion thought it was too smart * Use map instead of vector so we can keep track even if they go out of sight * Fix launch files, remove dead code * Split processing into separate file * Add SE3, add filter, slightly modify clang format, restructure perception * Filtered positioning for arucos in 3D space working, refactoring * Fix formatting * Refactor out SE3 transform function * Add more to SE3, continue refactor * Name median mean filter, cleanup se3 names * Use eigen for implementation of se3 * Use tf tree instead of matrix multiplication * Minor documentation * Update README.md
Closes #18